Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: afterCreateUser callback being called before setting user's roles #30309

Merged
merged 17 commits into from
Apr 23, 2024

Conversation

KevLehman
Copy link
Contributor

@KevLehman KevLehman commented Sep 6, 2023

Proposed changes (including videos or screenshots)

Issue(s)

https://rocketchat.atlassian.net/browse/OC-1276
https://rocketchat.atlassian.net/browse/SUP-541

Steps to test or reproduce

Further comments

This fixes a problem with Business Hours, where the callback for updating agents and assign the proper open business hours was not being fired because afterCreateUser was being called way before setting the roles for a user.

This caused the callback to ignore the user and thus not updating its openBusinessHours, making the user unavailable to enable its status.

@KevLehman KevLehman changed the title callback being called fix: afterCreateUser callback being called before setting user's roles Sep 6, 2023
@KevLehman KevLehman force-pushed the fix/after-create-user-roles branch from 2cf82c6 to 32e0068 Compare September 6, 2023 17:06
@KevLehman KevLehman changed the base branch from SUP-349 to develop September 6, 2023 17:06
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.84%. Comparing base (1e772e4) to head (d9474b0).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #30309      +/-   ##
===========================================
+ Coverage    51.16%   54.84%   +3.68%     
===========================================
  Files         2026     2276     +250     
  Lines        46638    50407    +3769     
  Branches      9486    10274     +788     
===========================================
+ Hits         23861    27646    +3785     
+ Misses       20562    20287     -275     
- Partials      2215     2474     +259     
Flag Coverage Δ
e2e 53.74% <ø> (+7.59%) ⬆️
unit 74.95% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@changeset-bot
Copy link

changeset-bot bot commented Sep 7, 2023

🦋 Changeset detected

Latest commit: d9474b0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/models Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@murtaza98 murtaza98 marked this pull request as ready for review September 22, 2023 07:50
@murtaza98 murtaza98 added this to the 6.5.0 milestone Sep 22, 2023
@murtaza98
Copy link
Contributor

Thanks @pierre-lehnen-rc 🙌 Appreciate your insights and help on this item :)

@KevLehman KevLehman removed this from the 6.5.0 milestone Nov 1, 2023
Copy link
Contributor

dionisio-bot bot commented Apr 8, 2024

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@KevLehman KevLehman requested a review from a team as a code owner April 8, 2024 16:43
@jessicaschelly jessicaschelly added stat: QA assured Means it has been tested and approved by a company insider and removed stat: needs QA labels Apr 12, 2024
@KevLehman KevLehman added this to the 6.8 milestone Apr 15, 2024
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Apr 15, 2024
@dionisio-bot dionisio-bot bot removed the stat: ready to merge PR tested and approved waiting for merge label Apr 15, 2024
@KevLehman KevLehman added the stat: ready to merge PR tested and approved waiting for merge label Apr 23, 2024
@kodiakhq kodiakhq bot merged commit d79ddf1 into develop Apr 23, 2024
39 of 41 checks passed
@kodiakhq kodiakhq bot deleted the fix/after-create-user-roles branch April 23, 2024 14:39

if (Match.test(user.globalRoles, [String]) && user.globalRoles.length > 0) {
globalRoles.push(...user.globalRoles);
user.globalRoles.map((role) => globalRoles.add(role));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user.globalRoles.forEach((role) => globalRoles.add(role));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad: omnichannel stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants